Fix BidirectionalRelay half-close handling - #41328
Closed
Eamon (Eamon2009) wants to merge 3 commits into
Closed
Conversation
Fix logical condition and handle EOF in relay.cpp
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts the Windows-side relay implementation (src/windows/common/relay.cpp) to better handle half-close semantics and prevent a CPU spin when ReadFile reports an EOF via BytesRead == 0.
Changes:
- Updated
BidirectionalRelayloop termination to wait for both directions to complete (changed||to&&). - Updated
ScopedMultiRelay::Runto treat synchronousReadFile(...)=TRUEwithBytesRead == 0as EOF and transition toEof.
Suppressed comments (1)
src/windows/common/relay.cpp:323
- With the loop now exiting only when both handles are null, hitting EOF on one side (where the code sets
LeftHandle = nullptr/RightHandle = nullptr) causes the relay to keep reading from the other side but skip the corresponding write (else if (RightHandle != nullptr)/else if (LeftHandle != nullptr)), effectively dropping data rather than draining it. To support TCP half-close correctly, consider tracking read-EOF state separately (e.g.,leftReadClosed/rightReadClosed) while keeping the HANDLE for writes, and only stopping writes on actual write failure.
if ((LeftHandle == nullptr) && (RightHandle == nullptr))
{
break;
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Eamon (Eamon2009)
marked this pull request as draft
August 12, 2026 18:10
Comment on lines
+321
to
324
| if (leftReadClosed && rightReadClosed) | ||
| { | ||
| break; | ||
| } |
Comment on lines
+298
to
+301
| bool leftReadPending = false; | ||
| bool rightReadPending = false; | ||
| bool leftReadClosed = false; | ||
| bool rightReadClosed = false; |
Change || to && so both directions drain before the relay exits.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/windows/common/relay.cpp:301
- BidirectionalRelay is declared with
_In_ HANDLE LeftHandle/RightHandle(non-null), and current call sites pass non-null handles. InitializingleftReadClosed/rightReadClosedfrom== nullptris therefore redundant/misleading; either assert the non-null contract here or (if nullptr is valid) the API should be_In_opt_and documented accordingly.
bool leftReadClosed = (LeftHandle == nullptr);
bool rightReadClosed = (RightHandle == nullptr);
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BidirectionalRelayexited as soon as one side closed (||), dropping any buffered data the other side still had to send. Changed to&&so both directions drain before exit, matching the guest-side fix inlocalhost_relay.cpp.This completes end-to-end half-close handling: a Windows client that calls
shutdown(SD_SEND)can now reliably receive the Linux server's response.#41326